fix: fail the app when CONFIG_SVC config load fails - #28

Research Review

Verified

• Premise (silent exit under set -e): Confirmed. Removing the || valid_exports="" guard in a scratch copy makes the new garbled-output test fail (rc=1, no [CONFIG] ERROR on stderr), so the test exercises the guard and is not vacuous. The same sabotage run shows the other 6 tests unaffected.
• Tests: Ran tests/test-entrypoint-config-load-failure.sh at head 45c771a: 7/7 pass. test-entrypoint-subpath.sh and test-entrypoint-setup-timeout.sh still pass 7/7. The comment line # Load environment variables from config service if configured that test-entrypoint-subpath.sh greps for is preserved.
• Passing paths exist: empty store (exit 0, empty stdout) logs has no parameters and reaches the next phase; stderr-only noise with empty stdout continues; success evals exports; no CONFIG_SVC skips the block. Rejection-only coverage is not the case here. The test runs the real extracted block with stubs, matching the architect's direction.
• set -e handling: x=$(cmd) && config_exit=0 || config_exit=$? is unchanged and correct. valid_exports=$(... | grep ...) || valid_exports="" is correct (a pipeline's status is grep's). config_err_output=$(cat ... || true) is safe. No other $(... | grep ...) assignment exists in the script (grep sweep, only line 250).
• Exit codes: 124 on timeout, $config_exit otherwise, 1 for garbled output, all before pip install, same precedent as the setup.sh timeout block (lines 302-314).
• Decisions A1/B/C/D: match birme's approval at Fail the application when CONFIG_SVC config load fails instead of continuing #26 (comment) (A1 empty store not a failure; B exit non-zero, /healthz 500 dropped; C no retry; D unchanged).

Contract diff

• Container entrypoint (scripts/docker-entrypoint.sh:203-268, base 541ab74): not an HTTP handler, but the runtime's failure mode changes. Config-load failure (timeout, non-zero exit, garbled output): before = logs ERROR and continues without env vars; after = exits 124/N/1 before pip install. Empty store: before = silent exit 1 (set -e on grep); after = logs and continues. Timing: failure now ends boot within the 60s timeout instead of proceeding. New failure mode for apps with an unreachable/deleted/expired-token store. Changed, approved by birme at the comment URL above (the architect triage correctly gated this as NEEDS_HUMAN first). The PR body states the consumer impact and that a release plus remake is a manual follow-up.
• The 60s timeout is pre-existing and not changed by this PR. It was not measured here; now that it is terminal rather than tolerated, a cold npx -y @osaas/cli@latest fetch exceeding 60s turns a previously-working boot into a restart loop. Worth verifying (see Risks).

Existing consumers

• Not a standards/compliance PR. Omitted otherwise, but the PR's consumer statement is present and accurate against the diff: no CONFIG_SVC/token = unchanged; empty store = now starts; unreachable/deleted/expired = now fails at boot.

Adjacent Stubs Detected

• Swept scripts/docker-entrypoint.sh for undefined/|| undefined/TODO/FIXME/HACK/empty catch/log-only handlers: none found. Line 214 ... || true on the token refresh is intentional (refresh failure keeps the existing token) and not a stub.

Orphaned Files Detected

• None. tests/test-entrypoint-config-load-failure.sh is a standalone script like its 5 siblings; but see the CI note below.

Escape Hatch Coverage

• scripts/docker-entrypoint.sh:236-267: new terminal-rejection branches (exit on timeout/non-zero/garbled). Legitimate-user reachability: a valid store with expired token or slow npx fetch lands here. Bypass: none claimed in code or PR body; none implemented. Remediation path is operator action (refresh-app-config, fix the binding) and the hint is printed. Passing-path tests: found (empty store, stderr noise, success, no CONFIG_SVC). No blocker.
• Gate premise: the gate inspects the real config-to-env stdout/exit code. The tests use a hand-built npx stub; I did not run the real @osaas/cli web config-to-env against a real store (no OSC token in this session), so the "empty store => exit 0 + empty stdout" premise rests on the architect's triage and the stub. Listed under Risks.

Object Shape Verification

• No fetched/ORM object spread in the diff (shell script only).

Needs Verification (non-blocking)

• scripts/docker-entrypoint.sh:257: the garbled-output error echoes the full $config_env_output to stderr (pod logs). In that branch no line matched the strict export grep, but lines like export my-key=<secret value> (the exact example the architect cited) would be printed verbatim with the value. Consider logging only line count / first N chars, or redacting values. Same for line 241 on non-zero exit, where stdout is echoed.
• scripts/docker-entrypoint.sh:243: hint regex 401|403|... is unanchored and matches any output containing those digits (e.g. a port or byte count), so the "token may have expired" hint can be misleading. Cosmetic.
• Empty-store premise via npm noise: if npx ever prints a banner to stdout on an empty store, that is classified as garbled and fatal. stdout/stderr split makes this unlikely (npm notices go to stderr), but unverified against the real CLI.
• CI: .github/workflows/ contains only the two fork-sync workflows and the PR shows no checks, so none of the tests/*.sh run in CI. The 7/7 result above is from my local run only; the new test guards nothing automatically after merge.

Impact Analysis

• Blast radius: all python-runner apps bound to a config store, at remake time only. Dev fork rebuilds automatically on merge; prod needs release + remake (human, not done here).
• Downstream consumers: osaas-deploy-manager checkBuildStatus will show a crash-looping app as building (per the architect triage, unverified on live ingress). validateConfigServiceReachable fails soft, so dead bindings can still be created and will now fail every boot.
• Sibling paths in the same script: no other config-load/grep-under-set -e instance in docker-entrypoint.sh. Other runners (web-runner#64 triaged separately, php-runner, wasm-runner) may share the original pattern; out of scope for this PR but not checked by me.
• Coordinated changes needed: release + remake of python-runner. Suggested: before remaking prod, run the Loki query from the triage to count apps that already silently crash on empty stores or have dead stores.
• Breaking change risk: medium for apps with dead/expired stores (approved), low otherwise.

Risks Not Tested

• Real @osaas/cli web config-to-env behaviour (exit code and stdout for an empty store, for an expired token, for 401) was not run; only stubs were used.
• Whether the 60s timeout is enough for a cold npx -y @osaas/cli@latest on Elastx; no latency measurement available. A slow registry fetch is now fatal.
• Deploy-manager status and ingress behaviour for a crash-looping pod (stays building) not confirmed live.
• Number of existing prod apps with dead/expired/empty stores (blast radius at remake) not quantified.
• Test script does not run in CI; the local pass is not an automated gate.

Research Suggestions

• Redact or truncate echoed config output at lines 241 and 257.
• Wire tests/test-entrypoint-*.sh into a CI workflow (separate ticket is fine).
• Run the Loki query from the Fail the application when CONFIG_SVC config load fails instead of continuing #26 triage before remaking prod.

Verdict

No blocking issues found.

Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.